Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ros2controlcli] add params file parsing to load_controller verb and add namespacing support #1703

Merged
merged 14 commits into from
Sep 11, 2024
Merged

[ros2controlcli] add params file parsing to load_controller verb and add namespacing support #1703

merged 14 commits into from
Sep 11, 2024

Conversation

saikishor
Copy link
Member

@saikishor saikishor commented Aug 20, 2024 •
edited
Loading

As mentioned in #1661, @christophfroehlich has mentioned that the ros2controlcli is lacking some features to load the parameters, etc. I've modified the ros2controlcli to be able to set the parameters from the command line and as well to be able to work with the namespaced controller managers etc

now you can load the controllers by passing the params file relative or absolute path and also be able to pass --ros-args
to each of the ros2controlcli verbs to be able to work with the namespaced controllers and controller_manager

ros2 control load_controller joint_state_broadcaster
ros2 control load_controller forward_position_controller config/rrbot_controllers.yaml --set-state active
ros2 control load_controller forward_position_controller /tmp/ws/src/ros2_control_demos/example_1/bringup/config/rrbot_controllers.yaml --set-state active
ros2 control load_controller forward_position_controller config/rrbot_controllers.yaml --set-state active --ros-args -r __ns:=namespace1

@codecov Codecov
Copy link

codecov bot commented Aug 20, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.71%. Comparing base (8552dcf) to head (9060f5b).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
.../controller_manager/controller_manager_services.py 83.33% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1703   +/-   ##
=======================================
  Coverage   86.71%   86.71%           
=======================================
  Files         116      116           
  Lines       10695    10697    +2     
  Branches      984      984           
=======================================
+ Hits         9274     9276    +2     
  Misses       1064     1064           
  Partials      357      357
Flag Coverage Δ
unittests 86.71% <83.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../controller_manager/controller_manager_services.py 70.49% <83.33%> (+0.49%) ⬆️

@christophfroehlich christophfroehlich mentioned this pull request Sep 7, 2024
Merged
christophfroehlich
christophfroehlich approved these changes Sep 7, 2024
View reviewed changes
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I tested the paramsfile successfully (but not the namespaceing).

@bmagyar bmagyar merged commit 4472287 into ros-controls:master Sep 11, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christophfroehlich christophfroehlich christophfroehlich approved these changes

@bmagyar bmagyar bmagyar approved these changes

@bijoua29 bijoua29 Awaiting requested review from bijoua29

@destogl destogl Awaiting requested review from destogl

@fmauch fmauch Awaiting requested review from fmauch

@livanov93 livanov93 Awaiting requested review from livanov93

@olivier-stasse olivier-stasse Awaiting requested review from olivier-stasse

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@saikishor @christophfroehlich @bmagyar